Skip to content

feat: add PATCH endpoint for partial SBOM group assignment updates#2450

Open
ctron wants to merge 3 commits into
guacsec:mainfrom
ctron:fix/sbom-group-patch-assignment
Open

feat: add PATCH endpoint for partial SBOM group assignment updates#2450
ctron wants to merge 3 commits into
guacsec:mainfrom
ctron:fix/sbom-group-patch-assignment

Conversation

@ctron

@ctron ctron commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add PATCH /v3/group/sbom-assignment endpoint for partial SBOM group assignment updates
  • Unlike the existing PUT endpoint (which replaces all assignments), PATCH preserves existing assignments and only adds/removes the specified groups
  • Uses cartesian product semantics: each SBOM in sbom_ids gets all add groups added and all remove groups removed
  • Idempotent: duplicate adds use ON CONFLICT DO NOTHING, non-existent removals are silently ignored
  • Updates ADR 00013 to document the new endpoint

Fixes: TC-5036

Test plan

  • cargo fmt — clean
  • cargo clippy — clean
  • 9 new integration tests covering: add, remove, combined add+remove, idempotency, preserving existing assignments (core regression test), error cases (invalid/nonexistent SBOM, invalid group), empty request

🤖 Generated with Claude Code

Summary by Sourcery

Add support for partially updating SBOM group assignments via a new PATCH API endpoint and document its behavior.

New Features:

  • Introduce PATCH /api/v3/group/sbom-assignment endpoint to partially update SBOM group-to-SBOM assignments using add/remove semantics while preserving existing groups.

Enhancements:

  • Implement service-layer logic for idempotent partial assignment updates, including validation and conflict handling.
  • Extend OpenAPI/utoipa schema and SBOM group ADR to describe the new PATCH request/response contract and semantics.

Tests:

  • Add integration test utilities and new test coverage for PATCH assignment behavior, including add/remove operations, idempotency, preservation of existing assignments, and error handling for invalid inputs.

@sourcery-ai

sourcery-ai Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Adds a new PATCH /v3/group/sbom-assignment endpoint for partial SBOM group assignment updates, including service-layer logic, request model, routing, documentation, and comprehensive integration tests.

Sequence diagram for PATCH sbom group assignment endpoint

sequenceDiagram
    actor User
    participant Endpoint_patch_assignments
    participant SbomGroupService
    participant Db

    User->>Endpoint_patch_assignments: PATCH /v3/group/sbom-assignment
    Endpoint_patch_assignments->>Db: begin
    Endpoint_patch_assignments->>SbomGroupService: patch_assignments(sbom_ids, add, remove, tx)

    SbomGroupService->>Db: sbom::Entity::update_many.exec
    SbomGroupService->>Db: sbom_group_assignment::Entity::delete_many.exec
    SbomGroupService->>Db: sbom_group_assignment::Entity::insert_many.on_conflict.do_nothing.exec

    SbomGroupService-->>Endpoint_patch_assignments: Result<(), Error>
    Endpoint_patch_assignments->>Db: commit
    Endpoint_patch_assignments-->>User: 204 NoContent
Loading

File-Level Changes

Change Details Files
Implement partial SBOM group assignment logic with idempotent add/remove semantics in the service layer.
  • Add SbomGroupService::patch_assignments to support partial updates over multiple SBOMs using add/remove group lists
  • Parse and validate SBOM and group IDs, returning NotFound for invalid SBOM IDs and BadRequest for invalid group IDs via FK checks
  • Bump SBOM revisions in bulk and ensure all requested SBOMs exist before applying changes
  • Delete existing assignments only for specified remove groups and insert new assignments for add groups using ON CONFLICT DO NOTHING to guarantee idempotency
modules/fundamental/src/sbom_group/service.rs
Expose the PATCH endpoint, request model, and OpenAPI annotations for partial assignment updates.
  • Define PatchAssignmentRequest struct with sbom_ids, add, and remove fields, defaulting add/remove to empty lists
  • Register patch_assignments handler in sbom_group endpoint configuration and implement transactional handling that delegates to the service method
  • Annotate the new PATCH route with utoipa metadata, including request body and response codes
  • Update ADR 00013 to describe the new PATCH /api/v3/group/sbom-assignment API contract, semantics, and error responses
modules/fundamental/src/sbom_group/model.rs
modules/fundamental/src/sbom_group/endpoints/mod.rs
docs/adrs/00013-sbom-groups.md
Add test utilities and integration tests to validate PATCH behavior, including idempotency and error handling.
  • Introduce PatchAssignments test helper to build and execute PATCH /api/v3/group/sbom-assignment requests with configurable add/remove and expected status
  • Extend assignment endpoint tests to cover add, remove, combined add+remove, idempotent behavior, and preservation of existing assignments (TC-5036 regression)
  • Add negative tests for nonexistent/invalid SBOM IDs, empty sbom_ids behavior, and invalid group IDs returning BAD_REQUEST
  • Wire PatchAssignments into the common test module and update imports accordingly
modules/fundamental/src/common/test.rs
modules/fundamental/src/sbom_group/endpoints/test/assignment.rs

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 2 issues, and left some high level feedback:

  • In patch_assignments, the insert_many call uses both .on_conflict(...).do_nothing() and a trailing .do_nothing(), which is redundant and can be simplified to rely on the configured OnConflict clause only.
  • The OpenAPI/utoipa annotation for patch_assignments only declares 204/400/401/403 but the service can return Error::NotFound, so consider adding a 404 response to the path metadata to align the API description with actual behavior.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `patch_assignments`, the insert_many call uses both `.on_conflict(...).do_nothing()` and a trailing `.do_nothing()`, which is redundant and can be simplified to rely on the configured `OnConflict` clause only.
- The OpenAPI/utoipa annotation for `patch_assignments` only declares 204/400/401/403 but the service can return `Error::NotFound`, so consider adding a 404 response to the path metadata to align the API description with actual behavior.

## Individual Comments

### Comment 1
<location path="modules/fundamental/src/sbom_group/endpoints/mod.rs" line_range="52" />
<code_context>
+        .service(patch_assignments);
 }

 #[utoipa::path(
</code_context>
<issue_to_address>
**issue (bug_risk):** The OpenAPI responses omit 404, even though the service can return `NotFound`.

`patch_assignments` can return `Error::NotFound` (e.g., when SBOM IDs fail to parse or SBOMs are missing), but the `utoipa::path` annotation only declares 204/400/401/403. This divergence between behavior and OpenAPI spec can mislead generated clients and integrators. Please either document a 404 response in the path or adjust the error handling to use one of the declared status codes.
</issue_to_address>

### Comment 2
<location path="modules/fundamental/src/sbom_group/endpoints/test/assignment.rs" line_range="446-455" />
<code_context>
+
+#[test_context(TrustifyContext)]
+#[test_log::test(actix_web::test)]
+async fn patch_sbom_group_assignments_add(ctx: &TrustifyContext) -> anyhow::Result<()> {
+    let app = caller(ctx).await?;
+
+    let group1: GroupResponse = Create::new("Group 1").execute(&app).await?;
+    let group2: GroupResponse = Create::new("Group 2").execute(&app).await?;
+
+    let sbom = ctx
+        .ingest_document("zookeeper-3.9.2-cyclonedx.json")
+        .await?;
+    let sbom_id = sbom.id.to_string();
+
+    PatchAssignments::new(vec![sbom_id.clone()])
+        .add_groups(vec![group1.id.clone(), group2.id.clone()])
+        .execute(&app)
+        .await?;
+
+    assert_assigned_groups(&app, &sbom_id, &[&group1.id, &group2.id]).await?;
+
+    Ok(())
</code_context>
<issue_to_address>
**suggestion (testing):** Add coverage for PATCH semantics when multiple SBOM IDs are passed (cartesian product behavior)

Current tests only cover a single SBOM, but the PATCH endpoint is documented to apply cartesian product semantics across all `sbom_ids` and `add`/`remove` groups. Please add at least one integration test using `PatchAssignments::new(vec![sbom1_id.clone(), sbom2_id.clone()])` with multiple groups in `add` and/or `remove`, and assert that each SBOM receives the expected assignments. This will verify the bulk semantics and help prevent regressions in multi-SBOM behavior.

Suggested implementation:

```rust
 // --- PATCH assignment tests ---
//
// Bulk PATCH assignment semantics: multiple SBOMs with multiple groups (cartesian product)
#[test_context(TrustifyContext)]
#[test_log::test(actix_web::test)]
async fn patch_sbom_group_assignments_bulk_add(ctx: &TrustifyContext) -> anyhow::Result<()> {
    let app = caller(ctx).await?;

    // create groups to be added
    let group1: GroupResponse = Create::new("Bulk Group 1").execute(&app).await?;
    let group2: GroupResponse = Create::new("Bulk Group 2").execute(&app).await?;

    // ingest two SBOMs
    let sbom1 = ctx.ingest_document("zookeeper-3.9.2-cyclonedx.json").await?;
    let sbom2 = ctx.ingest_document("zookeeper-3.9.2-cyclonedx.json").await?;

    let sbom1_id = sbom1.id.to_string();
    let sbom2_id = sbom2.id.to_string();

    // apply PATCH with multiple sbom_ids and multiple groups; should be cartesian product
    PatchAssignments::new(vec![sbom1_id.clone(), sbom2_id.clone()])
        .add_groups(vec![group1.id.clone(), group2.id.clone()])
        .execute(&app)
        .await?;

    // verify each SBOM has both groups assigned
    assert_assigned_groups(&app, &sbom1_id, &[&group1.id, &group2.id]).await?;
    assert_assigned_groups(&app, &sbom2_id, &[&group1.id, &group2.id]).await?;

    Ok(())
}

// --- PATCH assignment tests ---

```

- This edit assumes `GroupResponse`, `Create`, `PatchAssignments`, and `assert_assigned_groups` are already imported in `assignment.rs`. If any are missing, add the appropriate `use` statements at the top of the file, consistent with existing conventions.
- If the test module uses specific naming or grouping/ordering conventions for tests (e.g., all `patch_*` tests grouped together), you may want to move `patch_sbom_group_assignments_bulk_add` next to the other PATCH assignment tests to keep the file organized.
- If there is an existing helper for creating multiple SBOMs or groups in tests, you can refactor the test to reuse that helper instead of calling `ingest_document` and `Create::new` directly, while keeping the core cartesian-product assertions unchanged.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread modules/fundamental/src/sbom_group/endpoints/mod.rs
Comment on lines +446 to +455
async fn patch_sbom_group_assignments_add(ctx: &TrustifyContext) -> anyhow::Result<()> {
let app = caller(ctx).await?;

let group1: GroupResponse = Create::new("Group 1").execute(&app).await?;
let group2: GroupResponse = Create::new("Group 2").execute(&app).await?;

let sbom = ctx
.ingest_document("zookeeper-3.9.2-cyclonedx.json")
.await?;
let sbom_id = sbom.id.to_string();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Add coverage for PATCH semantics when multiple SBOM IDs are passed (cartesian product behavior)

Current tests only cover a single SBOM, but the PATCH endpoint is documented to apply cartesian product semantics across all sbom_ids and add/remove groups. Please add at least one integration test using PatchAssignments::new(vec![sbom1_id.clone(), sbom2_id.clone()]) with multiple groups in add and/or remove, and assert that each SBOM receives the expected assignments. This will verify the bulk semantics and help prevent regressions in multi-SBOM behavior.

Suggested implementation:

 // --- PATCH assignment tests ---
//
// Bulk PATCH assignment semantics: multiple SBOMs with multiple groups (cartesian product)
#[test_context(TrustifyContext)]
#[test_log::test(actix_web::test)]
async fn patch_sbom_group_assignments_bulk_add(ctx: &TrustifyContext) -> anyhow::Result<()> {
    let app = caller(ctx).await?;

    // create groups to be added
    let group1: GroupResponse = Create::new("Bulk Group 1").execute(&app).await?;
    let group2: GroupResponse = Create::new("Bulk Group 2").execute(&app).await?;

    // ingest two SBOMs
    let sbom1 = ctx.ingest_document("zookeeper-3.9.2-cyclonedx.json").await?;
    let sbom2 = ctx.ingest_document("zookeeper-3.9.2-cyclonedx.json").await?;

    let sbom1_id = sbom1.id.to_string();
    let sbom2_id = sbom2.id.to_string();

    // apply PATCH with multiple sbom_ids and multiple groups; should be cartesian product
    PatchAssignments::new(vec![sbom1_id.clone(), sbom2_id.clone()])
        .add_groups(vec![group1.id.clone(), group2.id.clone()])
        .execute(&app)
        .await?;

    // verify each SBOM has both groups assigned
    assert_assigned_groups(&app, &sbom1_id, &[&group1.id, &group2.id]).await?;
    assert_assigned_groups(&app, &sbom2_id, &[&group1.id, &group2.id]).await?;

    Ok(())
}

// --- PATCH assignment tests ---
  • This edit assumes GroupResponse, Create, PatchAssignments, and assert_assigned_groups are already imported in assignment.rs. If any are missing, add the appropriate use statements at the top of the file, consistent with existing conventions.
  • If the test module uses specific naming or grouping/ordering conventions for tests (e.g., all patch_* tests grouped together), you may want to move patch_sbom_group_assignments_bulk_add next to the other PATCH assignment tests to keep the file organized.
  • If there is an existing helper for creating multiple SBOMs or groups in tests, you can refactor the test to reuse that helper instead of calling ingest_document and Create::new directly, while keeping the core cartesian-product assertions unchanged.

@ctron ctron requested a review from rh-jfuller July 6, 2026 11:13
@ctron ctron requested review from carlosthe19916 and mrrajan July 6, 2026 11:13
@ctron ctron force-pushed the fix/sbom-group-patch-assignment branch from 9c2c6c0 to e0f82b4 Compare July 6, 2026 11:14
ctron added a commit to ctron/trustify-ui that referenced this pull request Jul 6, 2026
Switch useAddSBOMsToGroupsMutation from the PUT bulk assignment endpoint
(which replaces all assignments) to the new PATCH endpoint that
preserves existing group assignments when adding an SBOM to a new group.

Depends on: guacsec/trustify#2450

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@codecov

codecov Bot commented Jul 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.69369% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.62%. Comparing base (5ac9f8c) to head (686d93e).

Files with missing lines Patch % Lines
modules/fundamental/src/sbom_group/service.rs 92.30% 1 Missing and 4 partials ⚠️
...odules/fundamental/src/sbom_group/endpoints/mod.rs 86.66% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2450      +/-   ##
==========================================
+ Coverage   71.51%   71.62%   +0.10%     
==========================================
  Files         453      453              
  Lines       27360    27470     +110     
  Branches    27360    27470     +110     
==========================================
+ Hits        19567    19675     +108     
+ Misses       6672     6662      -10     
- Partials     1121     1133      +12     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ctron ctron added the backport release/0.5.z Backport (0.5.z) label Jul 6, 2026
@ctron ctron enabled auto-merge July 7, 2026 09:52
@ctron ctron force-pushed the fix/sbom-group-patch-assignment branch from 686d93e to f3f9c32 Compare July 7, 2026 09:53

@rh-jfuller rh-jfuller left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

apart from already observed comments, LGTM

@ctron ctron disabled auto-merge July 7, 2026 11:49
@ctron

ctron commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

Regarding the "redundant .do_nothing()" observation from the Sourcery review:

These are two different .do_nothing() calls on different types and both are necessary:

  1. OnConflict::do_nothing() (line 781) — configures the conflict resolution strategy on the OnConflict builder (i.e., "on conflict, do nothing").
  2. Insert::do_nothing() (line 784) — tells SeaORM's InsertStatement to not error when all rows are skipped (i.e., when the insert produces zero affected rows due to conflicts).

Without the second .do_nothing() on the Insert, SeaORM would return a RecordNotInserted error if every row in the batch already existed. Both calls serve distinct purposes.

@mrrajan

mrrajan commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

LGTM - verified with the automated scripts on local deployment: https://github.com/mrrajan/trustify-ui/blob/TC-3811/e2e/tests/api/features/sbom-groups.ts#L907-L1118

@ctron

ctron commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

Ok, I addressed the comments.

ctron and others added 3 commits July 7, 2026 16:17
The existing PUT endpoint uses replace semantics — assigning an SBOM to
a new group removes it from all previous groups. This adds a PATCH
endpoint (`PATCH /v3/group/sbom-assignment`) that supports additive and
subtractive assignment changes, preserving existing assignments.

Uses cartesian product semantics: each SBOM in sbom_ids gets all add
groups added and all remove groups removed. Idempotent via ON CONFLICT
DO NOTHING for adds and targeted DELETE for removes.

Fixes: TC-5036

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Address review comments: declare 404 in the OpenAPI annotation for
patchSbomGroupAssignments, and add a multi-SBOM PATCH test to verify
cartesian product semantics.

Assisted-by: Claude Code

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@ctron ctron force-pushed the fix/sbom-group-patch-assignment branch from 0be4083 to eba231a Compare July 7, 2026 14:17
@ctron ctron enabled auto-merge July 7, 2026 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport release/0.5.z Backport (0.5.z)

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

4 participants